[xcvr]: Support 150GHz grid in get_laser_config_freq/set_laser_freq#718
Open
gs1571 wants to merge 2 commits into
Open
[xcvr]: Support 150GHz grid in get_laser_config_freq/set_laser_freq#718gs1571 wants to merge 2 commits into
gs1571 wants to merge 2 commits into
Conversation
* get_freq_grid() already maps GridSpacing code 8 to 150GHz, but get_laser_config_freq() and set_laser_freq() were never updated to match: get_laser_config_freq() fell through to the generic branch and computed channel * 150 instead of channel * 25, and set_laser_freq() had no grid==150 case at all and returned False for any 150GHz-only module. * get_laser_config_freq(): treat grid 150 like grid 75 (channel step is 25GHz for both; 150GHz channels are simply every 6th 25GHz step). * set_laser_freq(): add a grid==150 branch, GridSpacingTx1 code 0x80, channel number required to be a multiple of 6. Signed-off-by: Grigory Solovyev <gs1571@gmail.com>
Collaborator
|
/azp run |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Collaborator
|
Hi, there are workflow run(s) waiting for approval, you may be first-time contributor. I will notify maintainers to help approve once PR is approved. Thanks! ---Powered by SONiC BuildBot
|
* OIF-CMIS 5.3 Table 8-66 defines the 150GHz grid as Frequency = 193.1 + (n+3) x 0.025 THz, unlike the 75GHz grid (Frequency = 193.1 + n x 0.025 THz) which has no additive offset. get_laser_config_freq()/set_laser_freq() treated both grids identically, so every 150GHz frequency was off by exactly +75GHz (3 channels x 25GHz) from what was requested. * get_laser_config_freq() now adds 3 to the channel before scaling for grid 150; set_laser_freq() subtracts 3 from the computed channel number before the "multiple of 6" validation and before writing it to the module. * Add/update tests in test_ccmis.py covering the corrected 150GHz formula and asserting the exact channel number written to the module. Signed-off-by: Grigory Solovyev <gs1571@gmail.com>
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
aditya-nexthop
approved these changes
Jul 20, 2026
Author
|
Hi @prgeor, reaching out to you specifically — saw you recently approved/merged related sonic_xcvr PRs (#701, #717, #722). This one is ready: CI green, no conflicts, approved by @aditya-nexthop on current HEAD, no open questions. Could you take a look when you have time? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
get_freq_grid()already mapsGridSpacingcode8to150(GHz), but thetwo functions that actually use the frequency grid for laser tuning were never
updated to match:
get_laser_config_freq()fell through to the genericelsebranch andcomputed
channel * 150instead ofchannel * 25— producing a frequencyup to several hundred GHz off, and sometimes outside the module's own
supported range.
set_laser_freq()had nogrid == 150branch at all and unconditionallyreturned
Falsefor any request on a 150GHz-only module.Fixed both:
get_laser_config_freq()now treats grid 150 the same as grid 75(channel step is 25GHz for both — a 150GHz channel is simply every 6th 25GHz
step).
set_laser_freq()gained agrid == 150branch (GridSpacingTx1code0x80, channel number required to be a multiple of 6).Motivation and Context
Found bringing up an 800G ZR+ coherent module (CMIS 5.3) that only supports
the 150GHz DWDM grid. Without this fix such a module can never be tuned to a
target frequency —
set_laser_freq()always fails, and even reading back theconfigured frequency is wrong.